Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(rpc): extend search path, remove cd #1137

Merged
merged 1 commit into from
Apr 1, 2021
Merged

Conversation

pnwamk
Copy link
Contributor

@pnwamk pnwamk commented Mar 26, 2021

Disable the ability for clients to change the current working directory of a running server - instead make it trivial to extend the search path dynamically. See #1081

  • add a test
  • verify the eval server is okay with this feature (it did not have the ability to change a working directory before... but extending the search path seems harmless and unlikely to violate eval server restrictions)
  • investigate if saw-remote-api also needs this feature

@pnwamk pnwamk self-assigned this Mar 26, 2021
@pnwamk pnwamk force-pushed the rpc/remove-change-cwd branch 2 times, most recently from 8e024a5 to 06454c7 Compare March 29, 2021 20:12
@pnwamk
Copy link
Contributor Author

pnwamk commented Mar 29, 2021

@atomb In this PR I've added the ability for clients to extend the CRYPTOLPATH the server uses to find modules for both cryptol-remote-api and cryptol-eval-server. I just want to double check this functionality doesn't conflict with the goals of the latter.

@pnwamk pnwamk marked this pull request as ready for review March 30, 2021 17:57
@pnwamk pnwamk requested a review from atomb March 31, 2021 21:55
Copy link
Contributor

@atomb atomb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me.

@pnwamk pnwamk merged commit 5dd9be0 into master Apr 1, 2021
@pnwamk pnwamk deleted the rpc/remove-change-cwd branch April 1, 2021 21:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants